src/sdk/namespace_s3.js (1)

126-132: Correct pagination field: use NextUploadIdMarker.

The response’s next marker is NextUploadIdMarker. Returning UploadIdMarker will regress pagination.

Apply this diff:

         return {
             objects: _.map(res.Uploads, obj => this._get_s3_object_info(obj, params.bucket)),
             common_prefixes: _.map(res.CommonPrefixes, 'Prefix'),
             is_truncated: res.IsTruncated,
             next_marker: res.NextKeyMarker,
-            next_upload_id_marker: res.UploadIdMarker,
+            next_upload_id_marker: res.NextUploadIdMarker,
         };

src/endpoint/s3/s3_utils.js (1)

671-673: Align error message with v3 shape and consider normalizing the return type.

• The message references "response header" while v3 exposes LastModified as a top-level field. Nit but confusing.
• Optional: consider returning a numeric timestamp for consistency with other paths that use getTime().

Apply this diff for the message:

-    if (!r.LastModified) throw new Error("date not found in response header");
+    if (!r.LastModified) throw new Error("LastModified not found on response");

src/sdk/namespace_s3.js (1)

862-896: Outdated JSDoc types for SSE fields (v2 types).

These reference AWS SDK v2 types. Consider updating to v3 inputs for clarity:

• ServerSideEncryption -> import("@aws-sdk/client-s3").ServerSideEncryption
• SSEKMSKeyId -> string, etc.

I can open a quick PR to refresh these annotations if you want.

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these settings in your CodeRabbit configuration.

src/sdk/object_sdk.js (1)

463-474: Restore signatureVersion selection to preserve SigV2 endpoints and SDK v2 fallback.

Without signatureVersion, we effectively force SDKv3/SigV4-only. Deployments/endpoints that still require SigV2 (e.g., resources configured with AWS_V2) will break because noobaa_s3_client.should_use_sdk_v2 won’t have a signal to choose SDK v2.

Apply this change:

                 s3_params: {
                     endpoint: r.endpoint,
                     aws_sts_arn: r.aws_sts_arn,
+                    signatureVersion: cloud_utils.get_s3_endpoint_signature_ver(r.endpoint, r.auth_method),
                     credentials: {
                         accessKeyId: r.access_key.unwrap(),
                         secretAccessKey: r.secret_key.unwrap(),
                     },
                     region: config.DEFAULT_REGION, // SDKv3 needs region
                     forcePathStyle: true,
                     requestHandler: noobaa_s3_client.get_requestHandler_with_suitable_agent(r.endpoint),
                     access_mode: r.access_mode,
                 },

src/sdk/object_sdk.js (2)

28-28: Nit: Consider lazy-requiring noobaa_s3_client to avoid top-level cost/cycles.

object_sdk is a hot path. Requiring the S3 client wrapper at module load time can increase startup cost and risks a circular require with NamespaceS3. Suggest moving this require into the S3 branch of _setup_single_namespace (just before creating NamespaceS3) to defer it until needed.

---

470-473: Region default and request handler: add safe fallback and ensure handler reuse.

• Region: Some S3-compatible endpoints expect us-east-1 (or ignore region). To reduce signature-mismatch risk when config is unset, prefer a safe fallback.
• Handler: Ensure get_requestHandler_with_suitable_agent caches/returns a shared NodeHttpHandler per endpoint (keep-alive, maxSockets) and respects HTTP(S)_PROXY env vars to avoid socket churn and honor proxies.

Suggested tweak for region:

-                    region: config.DEFAULT_REGION, // SDKv3 needs region
+                    region: r.region || config.DEFAULT_REGION || 'us-east-1', // SDKv3 needs region; fall back safely

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

src/endpoint/s3/s3_utils.js (1)

676-678: Broaden HTTP status acceptance to any 2xx (SDK v3 returns 206 for ranged GET/HEAD).

Only allowing 200 will break ranged operations and some HEADs under v3.

Apply this diff:

-function get_http_response_from_resp(res) {
-    if (res.$metadata.httpStatusCode !== 200) throw new Error("Response return with error");
-    return res;
-}
+function get_http_response_from_resp(res) {
+    const code = res?.$metadata?.httpStatusCode;
+    if (!code || Math.floor(code / 100) !== 2) {
+        throw new Error(`Response returned with error (status ${code})`);
+    }
+    return res;
+}

src/sdk/namespace_s3.js (1)

480-489: Don’t stringify PartNumberMarker; pass number or undefined.

S3 expects a number. Stringifying is unnecessary and can be surprising.

-        const req = {
+        const req = {
             Bucket: this.bucket,
             Key: params.key,
             UploadId: params.obj_id,
             MaxParts: params.max,
-            PartNumberMarker: params.num_marker?.toString(),
+            PartNumberMarker: params.num_marker,
         };

src/sdk/object_sdk.js (1)

466-475: Restore SigV2 support in ObjectSDK S3 client parameters

You’ve removed the signatureVersion setting from src/sdk/object_sdk.js, which means all S3 clients will default to SigV4—even IBM_COS endpoints that require SigV2 will break. Re-add the signature selector based on r.auth_method:

• File: src/sdk/object_sdk.js
Location: inside the s3_params object for NamespaceS3 (around line 471)

Suggested patch:

                     forcePathStyle: true,
                     requestHandler: noobaa_s3_client.get_requestHandler_with_suitable_agent(r.endpoint),
                     requestChecksumCalculation: 'WHEN_REQUIRED',
+                    signatureVersion: cloud_utils.get_s3_endpoint_signature_ver(r.endpoint, r.auth_method),
                     access_mode: r.access_mode,

This restores the V2/V4 switch via cloud_utils.get_s3_endpoint_signature_ver. If SigV2 is truly deprecated across all pools, remove IBM_COS from config.DEFAULT_S3_AUTH_METHOD and update docs/tests to reflect a SigV4-only policy.

src/endpoint/s3/s3_utils.js (1)

671-673: Clarify error and keep semantics consistent (LastModified).

The function now relies on LastModified, so the error message should reflect that. Callers currently expect a Date (not a string), which is fine.

-    if (!r.LastModified) throw new Error("date not found in response header");
+    if (!r.LastModified) throw new Error("LastModified not found in response");
     return r.LastModified;

src/sdk/namespace_s3.js (5)

32-43: Use v3 credentials shape when setting access key.

With v3 you’re passing credentials under s3_params.credentials, so this.access_key will be undefined and server-side copy checks will always fail.

-        this.access_key = s3_params.accessKeyId;
+        this.access_key = s3_params?.credentials?.accessKeyId || s3_params.accessKeyId;

---

249-256: Tighten request typing: only GetObjectRequest is used.

JSDoc union includes CopyObjectRequest which isn’t relevant to read path.

-        /** @type {import("@aws-sdk/client-s3").HeadObjectRequest & import("@aws-sdk/client-s3").GetObjectRequest | import("@aws-sdk/client-s3").CopyObjectRequest} */
+        /** @type {import("@aws-sdk/client-s3").HeadObjectRequest & import("@aws-sdk/client-s3").GetObjectRequest} */
         const request = {

---

265-266: Superfluous status guard (v3 throws on non-2xx).

The extra check is redundant and can hide null-safety issues; removing it simplifies the flow.

-            // In v3, non-2xx typically throws; keep this guard harmless.
-            if (obj_out.$metadata.httpStatusCode >= 300) throw new Error(`S3 getObject failed with status ${obj_out.$metadata.httpStatusCode}`);
+            // v3 throws on non-2xx; no additional guard needed

---

361-369: Head the created version and normalize last_modified_time.

Use the returned VersionId (if present) to head the exact created version. Also return epoch millis for consistency with other paths.

-        /** @type {import("@aws-sdk/client-s3").HeadObjectRequest} */
-        const request = {
-            Bucket: this.bucket,
-            Key: params.key,
-            VersionId: params.version_id,
-        };
-        const res_head = await this.s3.headObject(request);
-        const last_modified_time = await s3_utils.get_http_response_date(res_head);
+        /** @type {import("@aws-sdk/client-s3").HeadObjectRequest} */
+        const request = {
+            Bucket: this.bucket,
+            Key: params.key,
+            VersionId: res.VersionId || params.version_id,
+        };
+        const res_head = await this.s3.headObject(request);
+        const last_modified_time = s3_utils.get_http_response_date(res_head)?.getTime?.();

---

852-866: Broaden error translation for v3 error shapes (name/$metadata).

err.code is often undefined in v3. Use err.name and HTTP status to preserve mappings.

-    _translate_error_code(params, err) {
-        if (err.code === 'NoSuchKey') err.rpc_code = 'NO_SUCH_OBJECT';
-        else if (err.code === 'NotFound') err.rpc_code = 'NO_SUCH_OBJECT';
-        else if (err.code === 'InvalidRange') err.rpc_code = 'INVALID_RANGE';
-        else if (params.md_conditions) {
-            const md_conditions = params.md_conditions;
-            if (err.code === 'PreconditionFailed') {
-                if (md_conditions.if_match_etag) err.rpc_code = 'IF_MATCH_ETAG';
-                else if (md_conditions.if_unmodified_since) err.rpc_code = 'IF_UNMODIFIED_SINCE';
-            } else if (err.code === 'NotModified') {
-                if (md_conditions.if_modified_since) err.rpc_code = 'IF_MODIFIED_SINCE';
-                else if (md_conditions.if_none_match_etag) err.rpc_code = 'IF_NONE_MATCH_ETAG';
-            }
-        }
-    }
+    _translate_error_code(params, err) {
+        const code = err?.code || err?.name;
+        const http = err?.$metadata?.httpStatusCode;
+        if (code === 'NoSuchKey' || code === 'NotFound' || http === 404) {
+            err.rpc_code = 'NO_SUCH_OBJECT';
+        } else if (code === 'InvalidRange' || http === 416) {
+            err.rpc_code = 'INVALID_RANGE';
+        } else if (params.md_conditions) {
+            const md_conditions = params.md_conditions;
+            if (code === 'PreconditionFailed' || http === 412) {
+                if (md_conditions.if_match_etag) err.rpc_code = 'IF_MATCH_ETAG';
+                else if (md_conditions.if_unmodified_since) err.rpc_code = 'IF_UNMODIFIED_SINCE';
+            } else if (code === 'NotModified' || http === 304) {
+                if (md_conditions.if_modified_since) err.rpc_code = 'IF_MODIFIED_SINCE';
+                else if (md_conditions.if_none_match_etag) err.rpc_code = 'IF_NONE_MATCH_ETAG';
+            }
+        }
+    }

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

src/endpoint/s3/s3_utils.js (1)

670-672: Accept any 2xx status (v3 returns 206 for ranged responses) and improve error message.

Only accepting 200 will incorrectly throw on valid 206 responses (e.g., ranged GET/HEAD). Also tweak the message grammar and add null-safety.

Apply this diff:

-function get_http_response_date(res) {
-    if (res.$metadata.httpStatusCode !== 200) throw new Error("Response return with error");
-    if (!res.LastModified) throw new Error("Date not found in response header");
-    return res.LastModified;
-}
+function get_http_response_date(res) {
+    const code = res?.$metadata?.httpStatusCode;
+    if (!code || Math.floor(code / 100) !== 2) {
+        throw new Error(`Response returned with error (status ${code})`);
+    }
+    if (!res?.LastModified) throw new Error("Date not found in response header");
+    return res.LastModified;
+}

src/sdk/namespace_s3.js (1)

480-489: Don’t stringify PartNumberMarker; allow number or undefined. Also note inconsistency with the summary.

• API expects a number; .toString() is unnecessary and will throw when num_marker is undefined.
• The AI summary says this was addressed, but the code still stringifies.

-            PartNumberMarker: params.num_marker?.toString(),
+            PartNumberMarker: params.num_marker,

src/sdk/namespace_s3.js (3)

24-29: JSDoc: make bucket required to match runtime validation.

Constructor throws if bucket is missing, but the type still marks it optional (bucket?). Align the doc to avoid confusion.

-     *      bucket?: string,
+     *      bucket: string,

---

249-257: Nit: narrow request typing and drop the redundant 2xx guard.

• The request here is only for getObject; the CopyObjectRequest in the union isn’t used.
• v3 throws on non-2xx by default—explicit httpStatusCode >= 300 check is unnecessary.

-        /** @type {import("@aws-sdk/client-s3").HeadObjectRequest & import("@aws-sdk/client-s3").GetObjectRequest | import("@aws-sdk/client-s3").CopyObjectRequest} */
+        /** @type {import("@aws-sdk/client-s3").GetObjectRequest} */
         const request = {
             Bucket: this.bucket,
             Key: params.key,
             Range: params.end ? `bytes=${params.start}-${params.end - 1}` : undefined,
             PartNumber: params.part_number,
         };
@@
-            // In v3, non-2xx typically throws; keep this guard harmless.
-            if (obj_out.$metadata.httpStatusCode >= 300) throw new Error(`S3 getObject failed with status ${obj_out.$metadata.httpStatusCode}`);
+            // v3 throws on non-2xx; no explicit httpStatusCode guard needed.

Also applies to: 264-266, 279-281

---

360-369: Use returned VersionId for HEAD and normalize last_modified_time to a number.

• If versioning is enabled, head the specific version you just wrote (res.VersionId) rather than params.version_id.
• Normalize last_modified_time to a number (ms) for consistency with other code paths that use .getTime().

-        /** @type {import("@aws-sdk/client-s3").HeadObjectRequest} */
-        const request = {
-            Bucket: this.bucket,
-            Key: params.key,
-            VersionId: params.version_id,
-        };
-        const res_head = await this.s3.headObject(request);
-        const last_modified_time = await s3_utils.get_http_response_date(res_head);
+        /** @type {import("@aws-sdk/client-s3").HeadObjectRequest} */
+        const request = {
+            Bucket: this.bucket,
+            Key: params.key,
+            VersionId: res.VersionId, // prefer the just-created version when available
+        };
+        const res_head = await this.s3.headObject(request);
+        const last_modified_time = s3_utils.get_http_response_date(res_head).getTime();

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

src/sdk/namespace_s3.js (2)

35-35: Access key extraction is incorrect for SDK v3 credentials

The current approach of extracting accessKeyId directly from s3_params will be undefined in SDK v3, as credentials are typically provided under s3_params.credentials. This could lead to incorrect server-side copy decisions when both namespaces incorrectly compare as undefined.

Apply this diff to properly extract the access key:

-        this.access_key = s3_params.accessKeyId;
+        this.access_key = s3_params.credentials?.accessKeyId || s3_params.accessKeyId;

Also verify this works with async credential providers if they're used in your setup.

---

494-494: Handle undefined num_marker correctly

When params.num_marker is undefined, calling .toString() will throw an error. The SDK v3 expects a string for PartNumberMarker, but it's better to omit the field when undefined.

Apply this diff to handle undefined values:

-            PartNumberMarker: params.num_marker?.toString(),
+            PartNumberMarker: params.num_marker ? params.num_marker.toString() : undefined,

src/sdk/namespace_s3.js (1)

365-366: Fix typo in error message

There's a typo in the error log message.

-                dbg.error(`upload_object: Object upload failed for bucket ${this.bucket} and 
-                    key : ${params.key}, with erro : `, err);
+                dbg.error(`upload_object: Object upload failed for bucket ${this.bucket} and 
+                    key : ${params.key}, with error : `, err);

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

src/sdk/namespace_s3.js (1)

488-497: Don’t stringify PartNumberMarker; pass number or undefined.

The API expects a number. Stringifying is unnecessary and may cause subtle issues. This also reintroduces a previously fixed issue.

-            PartNumberMarker: params.num_marker?.toString(),
+            PartNumberMarker: params.num_marker,

src/sdk/namespace_s3.js (2)

24-29: Doc types: make bucket required in the constructor JSDoc.

Code enforces a required bucket, but JSDoc marks it optional. Align the type to prevent misuse.

-     *      s3_params: import("@aws-sdk/client-s3").S3ClientConfig & {
+     *      s3_params: import("@aws-sdk/client-s3").S3ClientConfig & {
      *          access_mode?: string,
      *          aws_sts_arn?: string,
      *      },
-     *      bucket?: string,
+     *      bucket: string,
      *      stats: import('./endpoint_stats_collector').EndpointStatsCollector,
      * }} args 

---

32-43: Access key extraction may be stale with STS/credential providers (affects server-side copy heuristics).

You capture access_key only from s3_params.credentials at construction. If STS is used in _prepare_sts_client (or a provider rotates credentials), this.access_key may not reflect the actual key used by this.s3, leading to false negatives/positives in is_server_side_copy.

Consider one of:

• Resolve and cache the current access key from the client provider when needed (e.g., a helper get_current_access_keyId() that awaits this.s3.config.credentials()).
• Or, relax the server-side copy heuristic to attempt CopyObject and fall back on AccessDenied, removing the potentially brittle key comparison.

If you want, I can provide a small helper that memoizes the resolved access key with a short TTL.

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

src/sdk/namespace_s3.js (1)

490-499: Pass numeric PartNumberMarker; don’t stringify.

The API expects a number. Converting to string is unnecessary. Keep it as a number or undefined.

-            PartNumberMarker: params.num_marker?.toString(),
+            PartNumberMarker: params.num_marker,

src/sdk/namespace_s3.js (5)

24-33: Make bucket required in JSDoc to match runtime check.

Constructor throws when bucket is missing, but the JSDoc marks it optional. Align the types to avoid confusion.

-     *      bucket?: string,
+     *      bucket: string,

---

249-256: Narrow the request type for read_object_stream; remove unrelated CopyObject type.

This method only performs getObject. The union with CopyObjectRequest (and even HeadObjectRequest) is misleading.

-        /** @type {import("@aws-sdk/client-s3").HeadObjectRequest & import("@aws-sdk/client-s3").GetObjectRequest | import("@aws-sdk/client-s3").CopyObjectRequest} */
+        /** @type {import("@aws-sdk/client-s3").GetObjectRequest} */
         const request = {
             Bucket: this.bucket,
             Key: params.key,
             Range: params.end ? `bytes=${params.start}-${params.end - 1}` : undefined,
             PartNumber: params.part_number,
         };

---

367-369: Fix log message typo and improve clarity.

Small nit and readability fix.

-                dbg.error(`upload_object: Object upload failed for bucket ${this.bucket} and 
-                    key : ${params.key}, with erro : `, err);
+                dbg.error(
+                    `upload_object: failed for bucket ${this.bucket}, key ${params.key}, error:`,
+                    err
+                );

---

810-816: Type union references look good for v3 shapes; minor doc nit.

Using import("@aws-sdk/client-s3")._Object et al. is accurate for v3. Consider avoiding leading-underscore types in JSDoc if you want to keep API surface stable (non-blocking).

---

888-912: JSDoc types reference v2 (AWS.S3.*); update to v3 equivalents.

The encryption-related JSDoc still points to v2 types. Replace with v3 S3 shapes (or just document field names) to avoid confusion.

Example:

-     *   ServerSideEncryption: AWS.S3.ServerSideEncryption,
-     *   SSEKMSKeyId: AWS.S3.SSEKMSKeyId,
-     *   SSECustomerKey: AWS.S3.SSECustomerKey,
-     *   SSECustomerAlgorithm: AWS.S3.SSECustomerAlgorithm,
-     *   CopySourceSSECustomerKey: AWS.S3.CopySourceSSECustomerKey,
-     *   CopySourceSSECustomerAlgorithm: AWS.S3.CopySourceSSECustomerAlgorithm,
+     *   ServerSideEncryption: import("@aws-sdk/client-s3").ServerSideEncryption,
+     *   SSEKMSKeyId: string,
+     *   SSECustomerKey: string,
+     *   SSECustomerAlgorithm: string,
+     *   CopySourceSSECustomerKey: string,
+     *   CopySourceSSECustomerAlgorithm: string,

src/sdk/object_sdk.js (1)

472-475: Optional: clarify interplay with NamespaceS3’s v3-only client creation.

NamespaceS3 unconditionally creates a v3 client via get_s3_client_v3_params. If any SigV2-only endpoints remain, they won’t be handled by this namespace regardless of signatureVersion. If that’s intentional per PR scope, ignore; otherwise consider routing SigV2 resources to an alternate namespace/client.

Would you like me to scan the repo for any remaining AWS_V2/SigV2-only configurations and list buckets/resources that could hit this path?

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

src/sdk/namespace_s3.js (1)

496-497: Remove unnecessary .toString() on PartNumberMarker
The params.num_marker?.toString() call is redundant—optional chaining already handles undefined, and other numeric params (e.g. MaxParts) are passed directly. Please update:

• File: src/sdk/namespace_s3.js, line 496

-            PartNumberMarker: params.num_marker?.toString(),
+            PartNumberMarker: params.num_marker,

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

src/sdk/namespace_s3.js (4)

214-221: V3-compatible InvalidRange handling: LGTM.

Falling back to headObject on err.name === 'InvalidRange' or httpStatusCode === 416 is correct for SDK v3.

---

270-284: Error propagation on stream: LGTM.

Wiring the source 'error' to the returned stream avoids silent failures. Good fix.

---

352-366: Command-scoped middleware for Date capture: LGTM.

Moving from client-level to command-level middleware removes race/leak hazards.

---

491-499: Do not stringify PartNumberMarker; pass number or leave undefined.

SDK expects a number. Stringifying is unnecessary and can produce subtle API errors.

-            PartNumberMarker: params.num_marker?.toString(),
+            PartNumberMarker: params.num_marker,

src/sdk/namespace_s3.js (8)

25-31: Make bucket required in JSDoc to match runtime validation.

Constructor throws when bucket is missing, but JSDoc marks it optional. Align the contract.

-     *      bucket?: string,
+     *      bucket: string,

---

36-39: Harden access key extraction for server-side copy checks.

In v3, credentials may be provided via a provider function or resolved on the client. Using only s3_params.credentials?.accessKeyId can yield undefined and cause incorrect is_server_side_copy decisions.

Apply this small reorder to derive from client config as a fallback (and tolerate provider functions):

-        this.access_key = s3_params.credentials?.accessKeyId;
         this.endpoint = s3_params.endpoint;
-        this.s3 = noobaa_s3_client.get_s3_client_v3_params(s3_params);
+        this.s3 = noobaa_s3_client.get_s3_client_v3_params(s3_params);
+        {
+            const creds = s3_params.credentials ?? this.s3?.config?.credentials;
+            this.access_key = typeof creds === 'function' ? undefined : creds?.accessKeyId;
+        }

If you intentionally rely only on explicit s3_params.credentials, please confirm so we can drop this suggestion.

---

191-198: Narrow the request type for clarity.

read_object_md builds either a HeadObjectRequest or GetObjectRequest. Keeping a union in JSDoc is fine, but the current combined type with a single object can confuse tooling.

-            /** @type {import("@aws-sdk/client-s3").HeadObjectRequest | import("@aws-sdk/client-s3").GetObjectRequest} */
+            /** @type {import("@aws-sdk/client-s3").HeadObjectRequest & Partial<import("@aws-sdk/client-s3").GetObjectRequest>} */

This keeps shared fields while acknowledging optional Range.

---

250-257: Fix JSDoc type: CopyObjectRequest is unrelated here.

This request is only for getObject; including CopyObjectRequest is misleading.

-        /** @type {import("@aws-sdk/client-s3").HeadObjectRequest & import("@aws-sdk/client-s3").GetObjectRequest | import("@aws-sdk/client-s3").CopyObjectRequest} */
+        /** @type {import("@aws-sdk/client-s3").GetObjectRequest} */

---

265-267: Remove redundant 3xx guard.

In v3, non-2xx responses throw. The explicit httpStatusCode >= 300 check won’t be reached and adds noise.

-            // In v3, non-2xx typically throws; keep this guard harmless.
-            if (obj_out.$metadata.httpStatusCode >= 300) throw new Error(`S3 getObject failed with status ${obj_out.$metadata.httpStatusCode}`);

---

367-369: Fix typo and improve error log formatting.

Minor nit: “erro” → “error”, and avoid multiline template literal with trailing spaces.

-                dbg.error(`upload_object: Object upload failed for bucket ${this.bucket} and 
-                    key : ${params.key}, with erro : `, err);
+                dbg.error(
+                    `upload_object: Object upload failed for bucket ${this.bucket} and key ${params.key}:`,
+                    err
+                );

---

862-876: Adapt error translation to v3 error shapes.

In v3, err.code is often unset. Prefer err.name and $metadata.httpStatusCode.

-    _translate_error_code(params, err) {
-        if (err.code === 'NoSuchKey') err.rpc_code = 'NO_SUCH_OBJECT';
-        else if (err.code === 'NotFound') err.rpc_code = 'NO_SUCH_OBJECT';
-        else if (err.code === 'InvalidRange') err.rpc_code = 'INVALID_RANGE';
+    _translate_error_code(params, err) {
+        const name = err?.code || err?.name;
+        const http = err?.$metadata?.httpStatusCode;
+        if (name === 'NoSuchKey' || name === 'NotFound' || http === 404) err.rpc_code = 'NO_SUCH_OBJECT';
+        else if (name === 'InvalidRange' || http === 416) err.rpc_code = 'INVALID_RANGE';
         else if (params.md_conditions) {
             const md_conditions = params.md_conditions;
-            if (err.code === 'PreconditionFailed') {
+            if (name === 'PreconditionFailed' || http === 412) {
                 if (md_conditions.if_match_etag) err.rpc_code = 'IF_MATCH_ETAG';
                 else if (md_conditions.if_unmodified_since) err.rpc_code = 'IF_UNMODIFIED_SINCE';
-            } else if (err.code === 'NotModified') {
+            } else if (name === 'NotModified' || http === 304) {
                 if (md_conditions.if_modified_since) err.rpc_code = 'IF_MODIFIED_SINCE';
                 else if (md_conditions.if_none_match_etag) err.rpc_code = 'IF_NONE_MATCH_ETAG';
             }
         }
     }

---

879-912: Update JSDoc encryption types to v3 or simplify to primitives.

Current annotations reference v2 AWS.S3.* types. In v3, these are simple string fields. Simplify to avoid confusion.

-    /** 
-     * @param {Partial<{
-     *   ServerSideEncryption: AWS.S3.ServerSideEncryption,
-     *   SSEKMSKeyId: AWS.S3.SSEKMSKeyId,
-     *   SSECustomerKey: AWS.S3.SSECustomerKey,
-     *   SSECustomerAlgorithm: AWS.S3.SSECustomerAlgorithm,
-     *   CopySourceSSECustomerKey: AWS.S3.CopySourceSSECustomerKey,
-     *   CopySourceSSECustomerAlgorithm: AWS.S3.CopySourceSSECustomerAlgorithm,
-     *  }>} request
-     */
+    /**
+     * @param {Partial<{
+     *   ServerSideEncryption: import('@aws-sdk/client-s3').ServerSideEncryption | string,
+     *   SSEKMSKeyId: string,
+     *   SSECustomerKey: string,
+     *   SSECustomerAlgorithm: string,
+     *   CopySourceSSECustomerKey: string,
+     *   CopySourceSSECustomerAlgorithm: string,
+     * }>} request
+     */

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

src/sdk/namespace_s3.js (2)

496-499: Don't stringify PartNumberMarker; pass number or leave undefined.

This was previously flagged; still using toString(). v3 expects a number; optional chaining avoids a throw but the type is still off.

-            PartNumberMarker: params.num_marker?.toString(),
+            PartNumberMarker: params.num_marker,

---

36-36: Access key extraction is too narrow; can be undefined in v3 (providers). Gate server-side copy accordingly.

In v3, credentials may come from a provider, so s3_params.credentials?.accessKeyId can be undefined. If both sides are undefined, current equality check erroneously allows server-side copy. Store access_key only when known and require truthy keys in the comparison.

-        this.access_key = s3_params.credentials?.accessKeyId;
+        // Only capture a static access key if present; avoid treating "both undefined" as equal later
+        this.access_key = (s3_params.credentials && 'accessKeyId' in s3_params.credentials)
+            ? s3_params.credentials.accessKeyId
+            : undefined;

src/sdk/namespace_s3.js (10)

29-31: JSDoc mismatch: bucket is enforced by ctor but marked optional.

Constructor throws when bucket is missing, so the JSDoc should declare bucket as required for accuracy and tooling.

- *      bucket?: string,
+ *      bucket: string,

---

87-94: Consider listObjectsV2 to align with AWS recommendations.

Not blocking, but v2 API (ListObjectsV2) is the modern, paginated call (ContinuationToken instead of Marker) and generally preferred.

---

250-257: Narrow request typing; CopyObjectRequest is unrelated here.

This path only calls getObject. Tighten the JSDoc to reduce confusion for maintainers and editors.

-        /** @type {import("@aws-sdk/client-s3").HeadObjectRequest & import("@aws-sdk/client-s3").GetObjectRequest | import("@aws-sdk/client-s3").CopyObjectRequest} */
+        /** @type {import("@aws-sdk/client-s3").GetObjectRequest} */

---

367-369: Typo in error log message.

Minor, but noisy in logs.

-                dbg.error(`upload_object: Object upload failed for bucket ${this.bucket} and 
-                    key : ${params.key}, with erro : `, err);
+                dbg.error(`upload_object: Object upload failed for bucket ${this.bucket} and key: ${params.key}, with error:`, err);

---

823-826: Avoid mutating res.Metadata when building xattr.

Safer to copy into a new object; prevents accidental mutation and handles undefined metadata.

-        const xattr = _.extend(res.Metadata, {
-            'noobaa-namespace-s3-bucket': this.bucket,
-        });
+        const xattr = Object.assign(
+            {},
+            res.Metadata || {},
+            { 'noobaa-namespace-s3-bucket': this.bucket }
+        );

---

234-241: Include err.name/http status in issues report for v3.

err.code may be empty in v3. Include name or http status to make the report useful.

-                object_sdk.rpc_client.pool.update_issues_report({
-                    namespace_resource_id: this.namespace_resource_id,
-                    error_code: String(err.code),
-                    time: Date.now(),
-                });
+                object_sdk.rpc_client.pool.update_issues_report({
+                    namespace_resource_id: this.namespace_resource_id,
+                    error_code: String(err.code || err.name || err?.$metadata?.httpStatusCode),
+                    time: Date.now(),
+                });

---

370-374: Same as above: enrich error_code for issues report.

-                object_sdk.rpc_client.pool.update_issues_report({
-                    namespace_resource_id: this.namespace_resource_id,
-                    error_code: String(err.code),
-                    time: Date.now(),
-                });
+                object_sdk.rpc_client.pool.update_issues_report({
+                    namespace_resource_id: this.namespace_resource_id,
+                    error_code: String(err.code || err.name || err?.$metadata?.httpStatusCode),
+                    time: Date.now(),
+                });

---

474-480: Same as above: enrich error_code for issues report.

-                object_sdk.rpc_client.pool.update_issues_report({
-                    namespace_resource_id: this.namespace_resource_id,
-                    error_code: String(err.code),
-                    time: Date.now(),
-                });
+                object_sdk.rpc_client.pool.update_issues_report({
+                    namespace_resource_id: this.namespace_resource_id,
+                    error_code: String(err.code || err.name || err?.$metadata?.httpStatusCode),
+                    time: Date.now(),
+                });

---

792-799: Same as above: enrich error_code for issues report.

-                object_sdk.rpc_client.pool.update_issues_report({
-                    namespace_resource_id: this.namespace_resource_id,
-                    error_code: String(err.code),
-                    time: Date.now(),
-                });
+                object_sdk.rpc_client.pool.update_issues_report({
+                    namespace_resource_id: this.namespace_resource_id,
+                    error_code: String(err.code || err.name || err?.$metadata?.httpStatusCode),
+                    time: Date.now(),
+                });

---

51-55: Comment typo.

“aboid” -> “avoid”.

-    // to aboid ACCESS_DENIED errors. a more complete solution is to always perform the server side copy
+    // to avoid ACCESS_DENIED errors. a more complete solution is to always perform the server side copy

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.